Add WSGI web server, socket listen, fix bugs #28
Conversation
- Added WSGI web server with up to 7 simultaneous connections, which are serviced sequentially - adafruit_wiznet5k_wsgiserver based on and has interface identical to adafruit_esp32spi_wsgiserver for maximum compatibility - Added socket 'listen' function - Fixed socket 'connected' logic and made compatible with 'listen' - Fixed source port > 65535 bug - Removed SOCKETS list, it was buggy and caused get_socket allocation to only work if socket connections ended in order, which is not the case for web servers (and wrong in many other cases) - Fixed TX buffer pointer limit to 16-bit - Allow 5s for DHCP response
|
Woah...awesome! I'm going to review this tomorrow. |
|
Is it possible to split out the wsgi server bit to work with any socket implementation? That way we'd be able to share it across all network providers. |
brentru
left a comment
brentru
left a comment
There was a problem hiding this comment.
Thanks for submitting this pull request. I have some questions along with some feedback.
I'm afraid that may not be possible, at least as far as I understand things. If you look at the ESP32 implementation for instance, it's very different from what I had to do here for the W5500. The ESP32 code has a "start server" call on the interface to listen on port 80, no socket involved at that point. I had to do a weird non-standard thing where I listen on port 80 on several sockets at the same time, something that would not be possible on a standard socket the way I understand things. But it was the only way I found to hold several connections at once until they can get served, instead of refusing connections after the first one is being served. I think a common interface at the WSGI level is as good as it's going to get. These chips all have different ways to do things and idiosyncrasies that may not be able to be resolved at the socket level. If you want to investigates ways to make that happen, be my guest, but it seems like it would be a huge undertaking, likely invalidated the next time another chip is added. |
- Server would hang and not serve any other clients when a client disconnected, fixed in `socket_write`. - Fixed `_get_tx_free_size` bug that would always return 0 and `socket_write` that used this function in a useless way and didn't actually check anything because it checked whether the returned bytearray was in a tuple of status ints. - `_get_tx_free_size` and `_get_rx_rcv_size` now return int instead of bytearray as you'd expect.
- The socket now expects `bind`, then `listen` for improved compatibility with CPython socket. - Fixed formatting to make black happy, even did the extremely ugly thing on line 222 of adafruit_wiznet5k_socket.py.
- Socket send now has a timeout so we can eventually release the socket in situations where we are trying to send but the network connection died so there was no TCP handshake to close the connection. - `WSGIServer` now has a 20s timeout for its sockets. - DHCP timeout increased to 30s in accordance with RFC3315. - Recognize more states as "socket is closed and available". - `get_socket` now returns an invalid value if no free socket is available, which will cause an exception in the socket constructor. It used to return 0 if no sockets were available, which is a valid socket value, and caused all kinds of problems for the rightful owner of socket 0.
|
Not sure what the automatic check is complaining about, I don't think I touched the files that fail? Is it black complaining about |
|
@brentru On it. Probably just gonna copy the license files over from master since that seems like an easier way to do it. |
|
Uhh, so I'm not sure why but now the CI isn't running at all. The only merge conflict left is one line in the license of adafruit_wiznet5k.py. @brentru I'm not really experienced enough with git to know exactly what the right thing to do here is. One possible solution I thought of was to just remove the copyright line that was added in this branch and then re-add it once this is merged. |
|
I did wonder about the license. The license file and README both call out LGPL but the sources files themselves say MIT. I did not make any change to this, but did wonder about the conflict there? |
|
@dherrada The CI won't run if you have merge conflicts. You'll need to do a |
|
@tannewt Ah, that makes sense. Good to know |
|
Any updates on this? I had actually started working on the same functionality and have a couple things I would like to add in after this is approved. |
|
@AdamCummick I just merged and released. Please feel free to open a separate PR with new functionalities and features. Thanks! |
|
Thanks @brentru ! |
5 participants
are serviced sequentially
adafruit_esp32spi_wsgiserver for maximum compatibility
to only work if socket connections ended in order, which is not the
case for web servers (and wrong in many other cases)